-
Notifications
You must be signed in to change notification settings - Fork 42
refactor : consolidate env variables #871
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add comprehensive paths.rs module with hierarchical path management - Consolidate workspace (.amazonq/), global (~/.aws/amazonq/), and application data paths - Replace hardcoded paths in tool_manager.rs, context.rs, and directories.rs - Use structured PathResolver with workspace(), global(), and application() scopes - Maintain backward compatibility while providing single source of truth for paths - Support async Os initialization and proper error handling - Suppress clippy warnings during migration phase This enables easier path management and future rebranding efforts by centralizing all Amazon Q related paths in one location. 🤖 Assisted by Amazon Q Developer
…ntrol Add SC2329 to the existing shellcheck disable comment for the __bp_adjust_histcontrol function in bash-preexec.sh. This function appears unused to shellcheck because it's overridden by a stub function, but it's actually called from within the __bp_install function. Fixes failing init lint tests for bash shell integrations. 🤖 Assisted by Amazon Q Developer
f5d2995 to
ac90afd
Compare
brandonskiser
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very minor nit - I would prefer just importing Env etc. instead of qualifying all callsites with fig_os_shim::..., more consistent with the rest of the codebase and more legible.
| static IN_CODESPACES: OnceLock<bool> = OnceLock::new(); | ||
| *IN_CODESPACES | ||
| .get_or_init(|| std::env::var_os("CODESPACES").is_some() || std::env::var_os("Q_CODESPACES").is_some()) | ||
| *IN_CODESPACES.get_or_init(|| std::env::var_os("CODESPACES").is_some() || crate::os::Env::new().in_codespaces()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Env::in_codespaces should also check for the "CODESPACES" env var right? So std::env::var_os("CODESPACES").is_some() shouldn't be necessary
|
|
||
| // Q-specific environment variable methods | ||
| pub fn q_fake_is_remote(&self) -> bool { | ||
| self.get_os("Q_FAKE_IS_REMOTE").is_some() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: rename get_os to get_os_env_var
Replace the deprecated CommandCargoExt::cargo_bin method with the recommended cargo_bin! macro to resolve compilation warnings. 🤖 Assisted by Amazon Q Developer
Replace deprecated assert_cmd::cargo::CommandCargoExt::cargo_bin method with the recommended cargo_bin! macro in test files to fix compilation warnings and ensure compatibility with custom cargo build directories. 🤖 Assisted by Amazon Q Developer
Refactor/consolidate env variables.